Skip to content

fix(overrides): emit clearing line for list-valued Ghostty directives#21

Merged
jakewan merged 1 commit into
mainfrom
feature/override-list-valued-directives
May 5, 2026
Merged

fix(overrides): emit clearing line for list-valued Ghostty directives#21
jakewan merged 1 commit into
mainfrom
feature/override-list-valued-directives

Conversation

@jakewan

@jakewan jakewan commented May 5, 2026

Copy link
Copy Markdown
Owner

Why

Ghostty treats some config directives as list-valued — each occurrence appends to the list rather than replacing. With font-family = X in the main config and font-family = Y in the override, both end up in Ghostty's font fallback chain with X as primary. The override never affects the rendered primary font, which made ghostty-switch font visibly broken whenever a user had a default font-family set in their main config.

Closes #20.

How

The override writer (internal/overrides/overrides.go) now knows the five list-valued Ghostty directives — font-family, font-family-bold, font-family-italic, font-family-bold-italic, keybind — and emits a clearing line before the value line for any of those keys present in the settings map:

font-family =
font-family = MartianMono Nerd Font

Ghostty processes the empty value as a list reset, then the subsequent line establishes the override as primary. Today only font-family is exercised, but encoding all five in the writer keeps Ghostty's directive semantics in one place — a future feature that writes keybind or the bold/italic variants gets the correct behavior for free.

Clearing is scoped to keys actually being written. The issue raised preemptive clearing of all known list-valued keys as a defensive option; rejected, because an unconditional keybind = would silently wipe any keybinds the user set in their main config (the override loads after).

Write is the single consolidation point — theme, font, and profile flows all funnel through it. No call-site changes.

Verification

  • just test passes, with new coverage in internal/overrides/overrides_test.go (three unit tests covering clearing-line emission for the single-key case, all five list-valued keys, and read/write round-trip preservation) and cmd/ghostty-switch/main_test.go (integration test asserting the profile-apply path produces the clearing+value pair).
  • just lint clean.
  • Manual end-to-end: with font-family = UbuntuMono Nerd Font in the main Ghostty config, ghostty-switch font (selecting Martian Mono) now produces an override file containing font-family =\nfont-family = MartianMono Nerd Font\n, and ghostty +show-config | rg font-family after reload shows only the override value.

Ghostty treats some config directives as list-valued: each occurrence
appends to the list rather than replacing. With `font-family = X` set in
the main config and `font-family = Y` in the override file, both end up
in the fallback chain with X as primary, so the override never affects
the rendered primary font.

Make the override writer aware of the five known list-valued keys
(font-family, font-family-bold, font-family-italic,
font-family-bold-italic, keybind) and emit a clearing line ("key =")
before the value line for any of those keys present in the settings map.
Ghostty processes the empty value as a list reset; the subsequent value
line establishes the override as primary.

Clearing is scoped to keys actually being written. Preemptive clearing
of unrelated list-valued keys would silently destroy any keybinds set
in the user's main config, since the override loads after.

Closes #20.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a Ghostty override semantics mismatch where certain directives are list-valued (append-on-repeat) rather than scalar (last-write-wins). By emitting a “clearing” directive line immediately before writing list-valued override keys, the override file reliably replaces earlier values from the main config (notably font-family), restoring expected behavior for ghostty-switch font and related flows.

Changes:

  • Teach internal/overrides.Write to emit key = clearing lines for known list-valued Ghostty directives before emitting key = value.
  • Add unit tests covering clearing-line emission and read/write round-trips for list-valued keys.
  • Add an integration test asserting the profile-apply path writes the clearing+value pair for font-family.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/overrides/overrides.go Adds awareness of list-valued Ghostty directives and emits a clearing line before writing those keys.
internal/overrides/overrides_test.go Adds unit coverage for clearing-line emission and round-trip behavior involving list-valued keys.
cmd/ghostty-switch/main_test.go Adds an integration test verifying profile application writes clearing+value for font-family.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

t.Fatalf("Write: %v", err)
}

data, _ := os.ReadFile(path)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred to #22 — the same data, _ := os.ReadFile(path) pattern exists in TestWriteIncludesHeader:67 and TestWriteSortsKeys:88 already, so the cleanup belongs in a scoped project-wide pass rather than bundled into this PR.

Comment on lines +171 to +172
data, _ := os.ReadFile(path)
return string(data)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred to #22 — same as above; tracked there for a project-wide pass across the four current instances in overrides_test.go.

@jakewan jakewan marked this pull request as ready for review May 5, 2026 17:46
@jakewan jakewan merged commit b315828 into main May 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Override mechanism cannot supersede list-valued Ghostty directives

2 participants